Skip to content

Preserve the .debug_gdb_scripts section #143679

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

sebastianpoeplau
Copy link

@sebastianpoeplau sebastianpoeplau commented Jul 9, 2025

Make sure that compiler and linker don't optimize the section's contents
away by adding the global holding the data to llvm.used. This
eliminates the need for a volatile load in the main shim; since the LLVM
codegen backend is the only implementer of the corresponding trait
function, remove it entirely.

Pretty printers in dylib dependencies are now emitted by the main crate
instead of the dylib; apart from matching how rlibs are handled, this
approach has the advantage that omit_gdb_pretty_printer_section keeps
working with dylib dependencies.

r? @bjorn3

@rustbot rustbot added A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 9, 2025
@rustbot
Copy link
Collaborator

rustbot commented Jul 9, 2025

Some changes occurred in compiler/rustc_codegen_ssa

cc @WaffleLapkin

Some changes occurred in compiler/rustc_codegen_gcc

cc @antoyo, @GuillaumeGomez

@sebastianpoeplau sebastianpoeplau force-pushed the preserve-debug-gdb-scripts-section branch 2 times, most recently from 97ac05e to 07de96b Compare July 9, 2025 12:16
@sebastianpoeplau sebastianpoeplau force-pushed the preserve-debug-gdb-scripts-section branch from 07de96b to d54c3ea Compare July 9, 2025 13:20
@bjorn3
Copy link
Member

bjorn3 commented Jul 9, 2025

@bors r+

@bors
Copy link
Collaborator

bors commented Jul 9, 2025

📌 Commit d54c3ea has been approved by bjorn3

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 9, 2025
tgross35 added a commit to tgross35/rust that referenced this pull request Jul 10, 2025
…b-scripts-section, r=bjorn3

Preserve the .debug_gdb_scripts section

Make sure that compiler and linker don't optimize the section's contents away by adding the global holding the data to `llvm.used`. This eliminates the need for a volatile load in the main shim; since the LLVM codegen backend is the only implementer of the corresponding trait function, remove it entirely.

r? `@bjorn3`
tgross35 added a commit to tgross35/rust that referenced this pull request Jul 10, 2025
…b-scripts-section, r=bjorn3

Preserve the .debug_gdb_scripts section

Make sure that compiler and linker don't optimize the section's contents away by adding the global holding the data to `llvm.used`. This eliminates the need for a volatile load in the main shim; since the LLVM codegen backend is the only implementer of the corresponding trait function, remove it entirely.

r? ``@bjorn3``
bors added a commit that referenced this pull request Jul 10, 2025
Rollup of 8 pull requests

Successful merges:

 - #140136 (Add an aarch64-msvc build running on ARM64 Windows)
 - #143642 (stdarch subtree update)
 - #143679 (Preserve the .debug_gdb_scripts section)
 - #143707 (Fix `--skip-std-check-if-no-download-rustc`)
 - #143722 (Make some "safe" llvm ops actually sound)
 - #143728 (Resolve refactor: extraction of `finalize_module_binding` and `single_import_can_define_name`)
 - #143742 (Rework borrowing suggestions to use `Expr` instead of just `Span`)
 - #143744 (Properly track the depth when expanding free alias types)

r? `@ghost`
`@rustbot` modify labels: rollup
@tgross35
Copy link
Contributor

Looks like some GDB tests failed #143755 (comment)
@bors r-
@bors2 try jobs=dist-i586-gnu-i586-i686-musl

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jul 11, 2025
rust-bors bot added a commit that referenced this pull request Jul 11, 2025
…ection, r=<try>

Preserve the .debug_gdb_scripts section

Make sure that compiler and linker don't optimize the section's contents away by adding the global holding the data to `llvm.used`. This eliminates the need for a volatile load in the main shim; since the LLVM codegen backend is the only implementer of the corresponding trait function, remove it entirely.

r? `@bjorn3`
try-job: dist-i586-gnu-i586-i686-musl
@rust-bors
Copy link

rust-bors bot commented Jul 11, 2025

⌛ Trying commit d54c3ea with merge f7ba1a6

To cancel the try build, run the command @bors2 try cancel.

@rust-bors
Copy link

rust-bors bot commented Jul 11, 2025

💔 Test failed

@sebastianpoeplau
Copy link
Author

Looks like some GDB tests failed #143755 (comment)

Interesting, I'm going to have a look. The test passes on my x86_64-unknown-linux-gnu; I'll try to find what's different in the CI job. The name dist-i586-gnu-i586-i686-musl suggests that it's a 32-bit build - where can I find out more? Is there a way to run the same setup locally?

@tgross35
Copy link
Contributor

You might be able to get away with passing --target ... to ./x since these tests should still be able to run on your host. It looks like it fails testing i686-unknown-linux-musl.

If that doesn't repro it for some reason you can replicate the CI setup pretty much exactly by running the docker tests https://rustc-dev-guide.rust-lang.org/tests/docker.html, with the downside that this takes about as long as CI does.

@sebastianpoeplau
Copy link
Author

Hmm, the full log of the CI job led me to believe that it's i586-unknown-linux-gnu, but locally the test passes for that target (running ./x.py test tests/debuginfo/embedded-visualizer.rs --target i586-unknown-linux-gnu). Will try with the musl target 👍

I noticed though that my GDB is version 15, whereas the test log shows GDB 12; maybe that's the reason for the discrepancy. In general, the .debug_gdb_scripts section looks correct to me...

@tgross35
Copy link
Contributor

You're right, it's i586. I must have been reading the wrong line.

##[group]Testing stage2 compiletest suite=debuginfo mode=debuginfo (x86_64-unknown-linux-gnu -> i586-unknown-linux-gnu)

I assume there probably isn't any reason we need to keep a very old gdb version if that makes a difference

@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Jul 26, 2025
bors added a commit that referenced this pull request Jul 26, 2025
…ection, r=bjorn3

Preserve the .debug_gdb_scripts section

Make sure that compiler and linker don't optimize the section's contents
away by adding the global holding the data to `llvm.used`. This
eliminates the need for a volatile load in the main shim; since the LLVM
codegen backend is the only implementer of the corresponding trait
function, remove it entirely.

Pretty printers in dylib dependencies are now emitted by the main crate
instead of the dylib; apart from matching how rlibs are handled, this
approach has the advantage that `omit_gdb_pretty_printer_section` keeps
working with dylib dependencies.

r? `@bjorn3`
@bors
Copy link
Collaborator

bors commented Jul 26, 2025

⌛ Testing commit 2133b4f with merge 09737a6...

@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Collaborator

bors commented Jul 26, 2025

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jul 26, 2025
@sebastianpoeplau sebastianpoeplau force-pushed the preserve-debug-gdb-scripts-section branch from 2133b4f to aec2912 Compare July 28, 2025 11:53
@sebastianpoeplau
Copy link
Author

Some more tests needed to be ignored on i586-unknown-linux-gnu due to the outdated linker. Sorry...

@bjorn3
Copy link
Member

bjorn3 commented Jul 28, 2025

@bors r+

@bors
Copy link
Collaborator

bors commented Jul 28, 2025

📌 Commit aec2912 has been approved by bjorn3

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 28, 2025
@sebastianpoeplau
Copy link
Author

The only case I can imagine this reducing debugability is when you use a rust dylib as plugin loaded with dlopen and this rust dylib uses #![debugger_visualizer(gdb_script_file = "...")].

Is this a common case? If so, or if we want to fix it nonetheless, then maybe the .debug_gdb_scripts sections could be made per-crate once the omit_gdb_pretty_printer_section tests don't cause problems anymore. That is, instead of collecting pretty printers transitively during compilation of executables/staticlibs/cdylibs, the compiler could collect pretty printers for the current crate only, relying on the .debug_gdb_scripts sections in Rust dylibs for dependencies 🤔

bors added a commit that referenced this pull request Jul 28, 2025
…ection, r=bjorn3

Preserve the .debug_gdb_scripts section

Make sure that compiler and linker don't optimize the section's contents
away by adding the global holding the data to `llvm.used`. This
eliminates the need for a volatile load in the main shim; since the LLVM
codegen backend is the only implementer of the corresponding trait
function, remove it entirely.

Pretty printers in dylib dependencies are now emitted by the main crate
instead of the dylib; apart from matching how rlibs are handled, this
approach has the advantage that `omit_gdb_pretty_printer_section` keeps
working with dylib dependencies.

r? `@bjorn3`
@bors
Copy link
Collaborator

bors commented Jul 28, 2025

⌛ Testing commit aec2912 with merge 3b8ad43...

@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Collaborator

bors commented Jul 28, 2025

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jul 28, 2025
@sebastianpoeplau
Copy link
Author

The x86_64-unknown-linux-musl CI image has the same problem as i586-unknown-linux-gnu: the linker is too old.

$ x86_64-linux-musl-ld --version
GNU ld (GNU Binutils) 2.33.1
Copyright (C) 2019 Free Software Foundation, Inc.
This program is free software; you may redistribute it under the terms of
the GNU General Public License version 3 or (at your option) a later version.
This program has absolutely no warranty.

Looking at musl-toolchain.sh it seems that we use https://github.com/richfelker/musl-cross-make to set up the musl toolchain. Our version of that repository is from April 2022; should I try using a more recent commit? Or disable the tests for x86_64-unknown-linux-musl as well? Or give up? 😅

@bors
Copy link
Collaborator

bors commented Jul 30, 2025

☔ The latest upstream changes (presumably #144673) made this pull request unmergeable. Please resolve the merge conflicts.

@bjorn3
Copy link
Member

bjorn3 commented Jul 31, 2025

That can be fixed later by replacing #![omit_gdb_pretty_printer_section] usage in the test suite with telling gdb to ignore .debug_gdb_scripts I think.

Opened #144738 for this.

@bjorn3
Copy link
Member

bjorn3 commented Jul 31, 2025

Our version of that repository is from April 2022; should I try using a more recent commit?

There is an open PR for updating musl, but it isn't something that can just be done due to some ABI breakage.

Maybe you could retain the volatile load inside main in addition to marking the global variable as used? For older toolchains the volatile load will be used for retaining the section in executables that use libstd, while for newer toolchains the used flag would be used for this instead for retaining the section in all executables.

@sebastianpoeplau
Copy link
Author

We would lose the advantage of being able to remove that code, but I guess it's our best option to stay compatible with old linkers 🤔 Will do.

Make sure that compiler and linker don't optimize the section's contents
away by adding the global holding the data to "llvm.used". The volatile
load in the main shim is retained because "llvm.used", which translates
to SHF_GNU_RETAIN on ELF targets, requires a reasonably recent linker;
emitting the volatile load ensures compatibility with older linkers, at
least when libstd is used.

Pretty printers in dylib dependencies are now emitted by the main crate
instead of the dylib; apart from matching how rlibs are handled, this
approach has the advantage that `omit_gdb_pretty_printer_section` keeps
working with dylib dependencies.
Instead of collecting pretty printers transitively when building
executables/staticlibs/cdylibs, let the debugger find each crate's
pretty printers via its .debug_gdb_scripts section. This covers the case
where libraries defining custom pretty printers are loaded dynamically.
@sebastianpoeplau sebastianpoeplau force-pushed the preserve-debug-gdb-scripts-section branch from aec2912 to 13f9889 Compare August 1, 2025 11:25
@sebastianpoeplau
Copy link
Author

I've changed the code to add the global to llvm.used in addition to the volatile load. The second commit drops the transitive collection of pretty printers and therefore depends on your fix for omit_gdb_pretty_printer_section in #144738.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-compiletest Area: The compiletest test runner A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. A-run-make Area: port run-make Makefiles to rmake.rs A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants